refactor to allow custom temporal dimension into time slider #1799

mayurmarakana89 · 2024-02-13T15:22:50Z

Description

Allows custom temporal dimension to existing layer path (single layer)

Fixes # (1540)

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.

Add the URL for your deploy!
https://mayurmarakana89.github.io/geoview/package-time-slider.html

Checklist:

I have build (rush build) and deploy (rush host) my PR
I have connected the issues(s) to this PR
My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
My changes generate no new warnings
I have created new issue(s) related to the outcome of this PR is needed
~~I have made corresponding changes to the documentation~~
~~I have added tests that prove my fix is effective or that my feature works~~
~~New and existing unit tests pass locally with my changes~~

This change is

jolevesq

Reviewed 1 of 4 files at r1, all commit messages.
Reviewable status: 1 of 4 files reviewed, 4 unresolved discussions (waiting on @Alex-NRCan, @amir-azma, @cphelefu, @DamonU2, @kaminderpal, @mayurmarakana89, and @ychoquet)

packages/geoview-core/public/configs/package-time-slider2-config-time-slider.json line 15 at r1 (raw file):

      "locked": true,
      "reversed": true,
      "defaultValue": "",

Like you said at the scrum... we may not need this and use the one from the dimension

packages/geoview-core/public/configs/package-time-slider2-config-time-slider.json line 18 at r1 (raw file):

      "temporalDimension": {
        "field": "datetime",
        "default": "1696-01-01T05:00:00Z",

Use array for default as it is dual handle [1950-01-01T05:00:00Z, 1970-01-01T05:00:00Z]
Set date from 1900-01-01T05:00:00Z to 2000-01-01T05:00:00Z and just put these 2 values

packages/geoview-core/public/configs/package-time-slider2-config-time-slider.json line 63 at r1 (raw file):

          "2016-01-01T04:47:32Z"
        ],
        "nearestValues": "discrete",

Should maybe be absolute

packages/geoview-core/src/core/stores/store-interface-and-intial-values/time-slider-state.ts line 179 at r1 (raw file):

      },
      // #endregion ACTIONS
      setTemporalDimension(layerPath: string, temporalDimension: TemporalDimensionProps | null): void {

The temporal dimension is set on the layer itself

https://canadian-geospatial-platform.github.io/geoview/public/package-time-slider.html
cgpv.api.maps['mapWM1'].layer.registeredLayers['historical-flood/0'].geoviewLayerInstance.layerTemporalDimension['historical-flood/0']

mayurmarakana89

Reviewable status: 0 of 6 files reviewed, 4 unresolved discussions (waiting on @Alex-NRCan, @amir-azma, @cphelefu, @DamonU2, @jolevesq, @kaminderpal, and @ychoquet)

packages/geoview-core/public/configs/package-time-slider2-config-time-slider.json line 15 at r1 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Like you said at the scrum... we may not need this and use the one from the dimension

Done.

packages/geoview-core/public/configs/package-time-slider2-config-time-slider.json line 18 at r1 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Use array for default as it is dual handle [1950-01-01T05:00:00Z, 1970-01-01T05:00:00Z]
Set date from 1900-01-01T05:00:00Z to 2000-01-01T05:00:00Z and just put these 2 values

Done.

packages/geoview-core/public/configs/package-time-slider2-config-time-slider.json line 63 at r1 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Should maybe be absolute

Done.

packages/geoview-core/src/core/stores/store-interface-and-intial-values/time-slider-state.ts line 179 at r1 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

The temporal dimension is set on the layer itself

https://canadian-geospatial-platform.github.io/geoview/public/package-time-slider.html
cgpv.api.maps['mapWM1'].layer.registeredLayers['historical-flood/0'].geoviewLayerInstance.layerTemporalDimension['historical-flood/0']

Done.

jolevesq

Reviewed 2 of 4 files at r2, 2 of 4 files at r3, all commit messages.
Reviewable status: 3 of 6 files reviewed, 2 unresolved discussions (waiting on @Alex-NRCan, @amir-azma, @cphelefu, @DamonU2, @kaminderpal, @mayurmarakana89, and @ychoquet)

packages/geoview-core/public/configs/package-time-slider2-config-time-slider.json line 21 at r3 (raw file):

        "range": {
          "type": "absolute",
          "range": [

Just put [1900-01-01T05:00:00Z to 2000-01-01T05:00:00Z] for range

mayurmarakana89

Reviewable status: 2 of 7 files reviewed, 2 unresolved discussions (waiting on @Alex-NRCan, @amir-azma, @cphelefu, @DamonU2, @jolevesq, @kaminderpal, and @ychoquet)

packages/geoview-core/public/configs/package-time-slider2-config-time-slider.json line 21 at r3 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Just put [1900-01-01T05:00:00Z to 2000-01-01T05:00:00Z] for range

Done.

jolevesq

Reviewed 1 of 4 files at r2, 2 of 4 files at r4, all commit messages.
Reviewable status: 5 of 7 files reviewed, 2 unresolved discussions (waiting on @Alex-NRCan, @amir-azma, @cphelefu, @DamonU2, @kaminderpal, @mayurmarakana89, and @ychoquet)

packages/geoview-core/src/core/stores/store-interface-and-intial-values/time-slider-state.ts line 11 at r4 (raw file):

};

// #region INTERFACES

Please, the start of region is few line above

packages/geoview-core/src/geo/layer/geoview-layers/abstract-geoview-layers.ts line 1503 at r4 (raw file):

  /** ***************************************************************************************************************************
   * Change the layer phase property and emit an event to update existing layer sets.

This line is not the good definition for the function

mayurmarakana89

Reviewable status: 3 of 7 files reviewed, 2 unresolved discussions (waiting on @Alex-NRCan, @amir-azma, @cphelefu, @DamonU2, @jolevesq, @kaminderpal, and @ychoquet)

packages/geoview-core/src/core/stores/store-interface-and-intial-values/time-slider-state.ts line 11 at r4 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Please, the start of region is few line above

Done.

packages/geoview-core/src/geo/layer/geoview-layers/abstract-geoview-layers.ts line 1503 at r4 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

This line is not the good definition for the function

Done.

mayurmarakana89 · 2024-02-20T18:38:06Z

Reviewed 1 of 4 files at r2, 2 of 4 files at r4, all commit messages.
Reviewable status: 5 of 7 files reviewed, 2 unresolved discussions (waiting on @Alex-NRCan, @amir-azma, @cphelefu, @DamonU2, @kaminderpal, @mayurmarakana89, and @ychoquet)

packages/geoview-core/src/core/stores/store-interface-and-intial-values/time-slider-state.ts line 11 at r4 (raw file):
};

// #region INTERFACES
Please, the start of region is few line above

packages/geoview-core/src/geo/layer/geoview-layers/abstract-geoview-layers.ts line 1503 at r4 (raw file):
  /** ***************************************************************************************************************************
   * Change the layer phase property and emit an event to update existing layer sets.
This line is not the good definition for the function

Done!

jolevesq

Reviewed 5 of 5 files at r5, all commit messages.
Reviewable status: 5 of 7 files reviewed, 2 unresolved discussions (waiting on @Alex-NRCan, @amir-azma, @cphelefu, @DamonU2, @kaminderpal, @mayurmarakana89, and @ychoquet)

packages/geoview-core/src/geo/layer/geoview-layers/abstract-geoview-layers.ts line 1508 at r6 (raw file):

   * @param {TimeDimension} temporalDimension The value to assign to the layer temporal dimension property.
   */
  setTemporalDimension(layerPath: string, temporalDimension: TimeDimension, override: boolean): void {

Add your new param with description to JSDOC function descriptoin

packages/geoview-core/src/geo/layer/geoview-layers/abstract-geoview-layers.ts line 1510 at r6 (raw file):

  setTemporalDimension(layerPath: string, temporalDimension: TimeDimension, override: boolean): void {
    layerPath = layerPath || this.layerPathAssociatedToTheGeoviewLayer;
    if (override) {

Shold not be the reverse... override would be when we pass custom dimension. Maybe use custom instead of overide for param anme

mayurmarakana89

Reviewable status: 5 of 7 files reviewed, 2 unresolved discussions (waiting on @Alex-NRCan, @amir-azma, @cphelefu, @DamonU2, @jolevesq, @kaminderpal, and @ychoquet)

packages/geoview-core/src/geo/layer/geoview-layers/abstract-geoview-layers.ts line 1508 at r6 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Add your new param with description to JSDOC function descriptoin

Done.

packages/geoview-core/src/geo/layer/geoview-layers/abstract-geoview-layers.ts line 1510 at r6 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Shold not be the reverse... override would be when we pass custom dimension. Maybe use custom instead of overide for param anme

Done.

jolevesq

Reviewed all commit messages.
Reviewable status: 5 of 7 files reviewed, 1 unresolved discussion (waiting on @Alex-NRCan, @amir-azma, @cphelefu, @DamonU2, @kaminderpal, @mayurmarakana89, and @ychoquet)

packages/geoview-core/src/geo/layer/geoview-layers/abstract-geoview-layers.ts line 1511 at r7 (raw file):

  setTemporalDimension(layerPath: string, temporalDimension: TimeDimension, custom: boolean): void {
    layerPath = layerPath || this.layerPathAssociatedToTheGeoviewLayer;
    if (custom) {

Should you look for !custom? because the way it is, your code will ends up in the old code. Can you re build and host...

mayurmarakana89

Reviewable status: 5 of 7 files reviewed, 1 unresolved discussion (waiting on @Alex-NRCan, @amir-azma, @cphelefu, @DamonU2, @jolevesq, @kaminderpal, and @ychoquet)

packages/geoview-core/src/geo/layer/geoview-layers/abstract-geoview-layers.ts line 1511 at r7 (raw file):

Previously, jolevesq (Johann Levesque) wrote…

Should you look for !custom? because the way it is, your code will ends up in the old code. Can you re build and host...

Done.

jolevesq

Reviewed 2 of 2 files at r8, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Alex-NRCan, @amir-azma, @cphelefu, @DamonU2, @kaminderpal, and @ychoquet)

mayurmarakana89 requested review from jolevesq, cphelefu, kaminderpal, ychoquet, DamonU2, Alex-NRCan and amir-azma February 13, 2024 15:36

jolevesq requested changes Feb 13, 2024

View reviewed changes

mayurmarakana89 force-pushed the 1628-time-slider-custom-layout branch 2 times, most recently from 21428f5 to acb81a5 Compare February 16, 2024 08:11

mayurmarakana89 requested a review from jolevesq February 16, 2024 14:14

mayurmarakana89 commented Feb 16, 2024

View reviewed changes

jolevesq requested changes Feb 16, 2024

View reviewed changes

mayurmarakana89 force-pushed the 1628-time-slider-custom-layout branch from e03e41b to 2265407 Compare February 20, 2024 11:29

mayurmarakana89 commented Feb 20, 2024

View reviewed changes

mayurmarakana89 requested a review from jolevesq February 20, 2024 11:37

jolevesq requested changes Feb 20, 2024

View reviewed changes

mayurmarakana89 force-pushed the 1628-time-slider-custom-layout branch from 4f98bbd to e2125bf Compare February 20, 2024 14:39

mayurmarakana89 requested a review from jolevesq February 20, 2024 14:39

mayurmarakana89 commented Feb 20, 2024

View reviewed changes

mayurmarakana89 force-pushed the 1628-time-slider-custom-layout branch from 1334d23 to 60fe467 Compare February 21, 2024 14:31

mayurmarakana89 added 8 commits February 22, 2024 11:42

refactor to allow custom temporal dimension into time slider

5021923

add time dimension config in custom config json

b50c948

code cleanup

50cce10

override time dimension with custom if preset

3d071e3

fix warnings

888afe8

fix range items autogenerated using createRangeOGC util

bfb4838

fix default value

b0108aa

remove default value logic

8c8dffd

mayurmarakana89 added 7 commits February 22, 2024 11:42

fix default value is array vs single value

a93d58d

fix default value override from slider config

4ea1548

fix layer data visibility on slider change

533ee60

fix default value data for spatiotemporal layer

035558d

fix default filter for custom temporal dimension for time slider

c396f93

fix default filters applied for map

eebf65b

override default value inside temporal dimension

9f727a4

mayurmarakana89 force-pushed the 1628-time-slider-custom-layout branch from e236932 to 9f727a4 Compare February 22, 2024 19:42

jolevesq requested changes Feb 22, 2024

View reviewed changes

add doc & rename variable

fdd8401

mayurmarakana89 commented Feb 22, 2024

View reviewed changes

jolevesq requested changes Feb 22, 2024

View reviewed changes

refactor to use custom config logic to time slider plugin

ebb608e

mayurmarakana89 commented Feb 22, 2024

View reviewed changes

jolevesq approved these changes Feb 22, 2024

View reviewed changes

Merge branch 'develop' into 1628-time-slider-custom-layout

675da06

jolevesq merged commit 9c931e2 into Canadian-Geospatial-Platform:develop Feb 22, 2024
6 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor to allow custom temporal dimension into time slider #1799

refactor to allow custom temporal dimension into time slider #1799

mayurmarakana89 commented Feb 13, 2024 •

edited

Loading

jolevesq left a comment

mayurmarakana89 left a comment

jolevesq left a comment

mayurmarakana89 left a comment

jolevesq left a comment

mayurmarakana89 left a comment

mayurmarakana89 commented Feb 20, 2024

jolevesq left a comment

mayurmarakana89 left a comment

jolevesq left a comment

mayurmarakana89 left a comment

jolevesq left a comment

refactor to allow custom temporal dimension into time slider #1799

refactor to allow custom temporal dimension into time slider #1799

Conversation

mayurmarakana89 commented Feb 13, 2024 • edited Loading

Description

Type of change

How Has This Been Tested?

Checklist:

jolevesq left a comment

Choose a reason for hiding this comment

mayurmarakana89 left a comment

Choose a reason for hiding this comment

jolevesq left a comment

Choose a reason for hiding this comment

mayurmarakana89 left a comment

Choose a reason for hiding this comment

jolevesq left a comment

Choose a reason for hiding this comment

mayurmarakana89 left a comment

Choose a reason for hiding this comment

mayurmarakana89 commented Feb 20, 2024

jolevesq left a comment

Choose a reason for hiding this comment

mayurmarakana89 left a comment

Choose a reason for hiding this comment

jolevesq left a comment

Choose a reason for hiding this comment

mayurmarakana89 left a comment

Choose a reason for hiding this comment

jolevesq left a comment

Choose a reason for hiding this comment

mayurmarakana89 commented Feb 13, 2024 •

edited

Loading